Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DPE-5374] Move agnostic upgrade to upgrade_helpers #481

Conversation

Gu1nness
Copy link
Contributor

Issue

  • The actual Upgrade class is substrate agnostic and should be reused.
  • This will allow testing the pre-upgrade checks on mongodb-k8s charm.

Solution

  • Extract in the upgrade_helpers.

…ting-vm-code-of-pre-upgrades-hook-on-k-8-s
Copy link
Contributor

@MiaAltieri MiaAltieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this and I acknowledge that the upgrade class is substrate agnostic.

But is it implementation agnostic. i.e. if we go with v3 on MongoDB K8s but v2 on MongoDB VM with this change still stand?

@Gu1nness
Copy link
Contributor Author

But is it implementation agnostic. i.e. if we go with v3 on MongoDB K8s but v2 on MongoDB VM with this change still stand?

This code does not care about the actual upgrade but only the checks.
If V3 needs some update, we can always overload the functions that would need a fix.
This only ports the minimal amount of code to make the pre upgrade hooks work :)

@MiaAltieri
Copy link
Contributor

MiaAltieri commented Sep 13, 2024

My concern is that functions in AbstractUpgrade won't be used in K8s , since ideally those upgrade specific functions will be in a lib for v3. But I trust you!

@Gu1nness Gu1nness merged commit d33d480 into 6/edge Sep 13, 2024
29 of 30 checks passed
@Gu1nness Gu1nness deleted the DPE-5374-mongo-db-k-8-s-upgrades-test-existing-vm-code-of-pre-upgrades-hook-on-k-8-s branch September 13, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants